Conversation
WalkthroughUpdated handle_agent_request to format the OpenTelemetry trace_id using format_trace_id and ensure session_id is a string. Tests were adjusted to mock tracer/span/format_trace_id and assert tracing behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Server as HTTP Server
participant Tracer as OTel Tracer
participant Span as Span
participant OTel as format_trace_id
participant Agent as AgentContext
Client->>Server: HTTP request
Server->>Tracer: start_span("handle_agent_request")
Tracer-->>Server: Span
Server->>Span: is_recording()
alt recording
Server->>Span: get_span_context().trace_id
Server->>OTel: format_trace_id(trace_id)
OTel-->>Server: formatted_trace_id
Server->>Agent: session_id = str(formatted_trace_id)
else not recording
Server->>Agent: session_id = None / existing logic
end
Server-->>Client: Response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
agentuity/server/__init__.py (2)
369-371: Wrong variable when extracting scope from nested x-agentuity-metadata.You’re checking “scope” in metadata, but the parsed JSON is in md.
Apply this diff:
- if "scope" in metadata: + if "scope" in md: scope = md["scope"] del md["scope"]
396-401: Same bug in top-level x-agentuity-metadata handling.Check md, not metadata.
Apply this diff:
- if "scope" in metadata: + if "scope" in md: scope = md["scope"] del md["scope"]
🧹 Nitpick comments (1)
tests/server/test_request_handlers.py (1)
102-106: Good: patch format_trace_id; also assert session_id is propagated.Strengthen the test by verifying the AgentContext.session_id passed to run_agent equals "test-trace-id".
Add after awaiting handle_agent_request:
# Verify session_id propagated to AgentContext mock_run_agent.assert_awaited() args, kwargs = mock_run_agent.call_args agent_context = args[-1] assert getattr(agent_context, "session_id") == "test-trace-id"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
agentuity/server/__init__.py(3 hunks)tests/server/test_request_handlers.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/server/test_request_handlers.py (3)
tests/server/test_agent.py (1)
mock_tracer(50-55)tests/server/test_agent_execution.py (1)
mock_tracer(18-23)tests/conftest.py (1)
mock_tracer(7-9)
🔇 Additional comments (2)
agentuity/server/__init__.py (2)
14-14: Import of format_trace_id is appropriate.This aligns with OTel’s API and enables stable 32-char hex IDs.
347-349: Correct: format trace_id to a string run_id.Using format_trace_id(...) ensures a canonical string run_id for propagation and logging.
| session_id=str(run_id), | ||
| scope=scope, | ||
| ) |
There was a problem hiding this comment.
Bug: session_id becomes the literal string "None" when tracing isn’t recording.
str(run_id) coerces None → "None". Downstream code will see a non-empty session_id, which is misleading.
Apply this diff:
- session_id=str(run_id),
+ session_id=run_id,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| session_id=str(run_id), | |
| scope=scope, | |
| ) | |
| session_id=run_id, | |
| scope=scope, | |
| ) |
🤖 Prompt for AI Agents
In agentuity/server/__init__.py around lines 450 to 452, session_id is set using
str(run_id) which converts None into the literal string "None" and misleads
downstream code; change this to pass None when run_id is None (e.g., session_id
= None if run_id is None else str(run_id)) so that an absent run_id remains a
true None rather than the string "None".
| # Create a mock span with proper trace_id formatting | ||
| mock_span_context = MagicMock() | ||
| mock_span_context.trace_id = 12345678901234567890123456789012345678 | ||
| mock_span = MagicMock() | ||
| mock_span.get_span_context.return_value = mock_span_context | ||
| mock_span.is_recording.return_value = True | ||
| mock_tracer = MagicMock() | ||
| mock_tracer.start_span.return_value.__enter__.return_value = mock_span | ||
|
|
There was a problem hiding this comment.
Mock the API you actually use: start_as_current_span, not start_span.
handle_agent_request uses tracer.start_as_current_span(...). Your mock wires start_span, so the configured span (is_recording, get_span_context) isn’t the one used.
Apply this diff:
- mock_tracer = MagicMock()
- mock_tracer.start_span.return_value.__enter__.return_value = mock_span
+ mock_tracer = MagicMock(spec=trace.Tracer)
+ mock_tracer.start_as_current_span.return_value.__enter__.return_value = mock_span📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Create a mock span with proper trace_id formatting | |
| mock_span_context = MagicMock() | |
| mock_span_context.trace_id = 12345678901234567890123456789012345678 | |
| mock_span = MagicMock() | |
| mock_span.get_span_context.return_value = mock_span_context | |
| mock_span.is_recording.return_value = True | |
| mock_tracer = MagicMock() | |
| mock_tracer.start_span.return_value.__enter__.return_value = mock_span | |
| # Create a mock span with proper trace_id formatting | |
| mock_span_context = MagicMock() | |
| mock_span_context.trace_id = 12345678901234567890123456789012345678 | |
| mock_span = MagicMock() | |
| mock_span.get_span_context.return_value = mock_span_context | |
| mock_span.is_recording.return_value = True | |
| - mock_tracer = MagicMock() | |
| mock_tracer = MagicMock(spec=trace.Tracer) | |
| mock_tracer.start_as_current_span.return_value.__enter__.return_value = mock_span |
🤖 Prompt for AI Agents
In tests/server/test_request_handlers.py around lines 91 to 99, the test mocks
tracer.start_span but the code under test calls tracer.start_as_current_span, so
the span used in the handler isn’t the mocked one; update the mock to set
mock_tracer.start_as_current_span.return_value.__enter__.return_value =
mock_span (and keep mock_span.get_span_context, mock_span.is_recording as is) so
the context manager used by start_as_current_span yields the configured mock
span.
- Add entry for v0.0.105 with session_id formatting fix - Include PR #91 reference - Add version comparison link Co-Authored-By: unknown <>
2 participants
Summary by CodeRabbit
Bug Fixes
Tests